Skip to content

Conversation

djc
Copy link
Contributor

@djc djc commented Oct 13, 2025

Followup from:

Sorry @FranciscoTGouveia, here are some more changes which IMO make things a bunch cleaner. I haven't done any testing yet (would be great if you want to do some of that) but I think the changes here are relatively straightforward?

@djc djc requested review from ChrisDenton and rami3l October 13, 2025 10:49
@djc djc force-pushed the download-cleanup branch from 3664131 to 360dd06 Compare October 13, 2025 11:40
Copy link
Member

@rami3l rami3l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo some questions :)

@rami3l
Copy link
Member

rami3l commented Oct 13, 2025

Please note that since this PR is related to more subtle changes in the user behavior that is not fully covered by the test suite, more attention should be paid to ensure the necessary preservation of original semantics.

@djc djc force-pushed the download-cleanup branch from 360dd06 to b96b51e Compare October 13, 2025 14:37
@djc djc force-pushed the download-cleanup branch from b96b51e to 9364884 Compare October 14, 2025 08:54
@djc djc force-pushed the download-cleanup branch from 9364884 to cc732d2 Compare October 14, 2025 11:07
@djc
Copy link
Contributor Author

djc commented Oct 16, 2025

Please note that since this PR is related to more subtle changes in the user behavior that is not fully covered by the test suite, more attention should be paid to ensure the necessary preservation of original semantics.

Do you think there's anything left to do here, or is it okay to merge this?

@rami3l
Copy link
Member

rami3l commented Oct 16, 2025

@djc I think it should be okay to merge this patch now that the concerns have been addressed. If there are further problems we can fix them separately later.

@djc djc added this pull request to the merge queue Oct 16, 2025
Merged via the queue into main with commit c59cd9c Oct 16, 2025
29 checks passed
@djc djc deleted the download-cleanup branch October 16, 2025 09:15
@djc djc mentioned this pull request Oct 16, 2025
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants